Fix failing YAML mapping IT #32851
Fix failing YAML mapping IT #32851
Conversation
Signed-off-by: Jeffrey Kinard <[email protected]>
|
R: @robertwb |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
| literal_int: 10 | ||
| named_field: element |
There was a problem hiding this comment.
Why is this order respected but the other ordering wasn't?
There was a problem hiding this comment.
There were in essence 2 issues (or 1 issue that required 2 fixes)
- Due to Schema inference parameterized types #32757 (for reasons I'm not entirely sure of), the ordering of the Row sent from the Java MapToFields transform was changed. To fix this, I reordered the expected output in the
AssertEqual - Because the change in 1) did not affect Python Rows, the
AssertEqualwould now fail for Python MapToFields since the order of the output Row did not change. To fix this, I changed the order of the fields in the config to reflect the new order in theAssertEqual
Ideally, the Java transform should not be changing the order (given that Python respects order, Java probably should too now that cross-language is so widespread), but I wanted to get tests green (and initially I thought this was a problem with the __eq__ being too restrictive with key order).
Perhaps more investigation should be done on #32757 to see why the ordering changed, or at the very least, I could open a bug about how Row ordering changes over Xlang.
There was a problem hiding this comment.
Looks like there was a change to sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/transforms/providers/JavaRowUdf.java in that PR (more specifically StaticSchemaInference.fieldFromType) which likely messed with how the schema was parsed in Java MapToFields
Actually that is per field, so it is more likely something else more tied to how expansion service parses the Schema. I can look into it next week if it is still an issue
Workaround for #32832
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.